Skip to content

WIP: implement modern MCP protocol conformance - #1341

Draft
adwsingh wants to merge 1 commit into
mainfrom
adwsingh/mcp-v2
Draft

WIP: implement modern MCP protocol conformance#1341
adwsingh wants to merge 1 commit into
mainfrom
adwsingh/mcp-v2

Conversation

@adwsingh

Copy link
Copy Markdown
Contributor

What behavior changes?

Describe the observable difference in behavior before and after this change.

Why is this change needed?

Explain the motivation: bug, feature request, refactor, performance, etc.

How was this validated?

List tests added, benchmarks run, or manual verification performed.

What should reviewers focus on?

Point reviewers to the files or sections that contain the interesting logic.

Additional Links

Related issues, design docs, or prior art.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@psp65

psp65 commented Aug 31, 2026

Copy link
Copy Markdown

Thanks for opening this up for testing, @adwsingh — this is exactly the kind of refactor we hoped for in #1337 (and it covers most of our proposed PR ladder plus the HTTP transport layer we'd deferred). We ran it end-to-end today: we adapted our gateway to McpEngine (mechanical, ~4 files — nice API to consume) and drove it with Python SDK 2.0.0 in all connect modes against a real proxied Smithy service behind the gateway, plus raw wire probes and byte-diffs against the pre-refactor engine's responses.

What works

  • All four SDK connect modes pass: mcp 1.28.1 legacy flow; 2.0.0 mode=legacy; mode=auto — which now adopts the modern era (discover → adopt, no initialize on the wire); and pinned mode="2026-07-28" including a tools/call through the full proxy pipeline (strict wire validation passes; resultType, _meta serverInfo, structuredContent all correct).
  • server/discover result is wire-complete; -32022 data shape is exact; era-conditional decoration puts ttlMs/cacheScope on exactly the cacheable set; the strict _meta envelope validation (-32602 without clientCapabilities) matches the reference SDK server; unknown methods stay -32601; the -32603 sanitized error handling (legacy too) and unknown-tool -32602 are nice improvements over what we had proposed.
  • prompts/list legacy responses are byte-identical to the pre-refactor engine.

Bugs found (in rough priority order)

1. Proxied tool names change (EchoEchoProxy) — breaks existing clients.
McpSchemaFactory.createTool derives the tool name from the operation schema's ShapeId (operationSchema.id().getName()), but for ProxyService-based services that's the synthesized <Operation>Proxy wrapper shape. The previous engine used operation.name(), which ProxyService sets to the delegate operation's name (see ProxyService: proxyOperation.getId().getName() passed as the server operation name). Result: every tool exposed through a proxied/bundled service is renamed, so any client that references tools by name breaks on upgrade. Descriptions inherit the same artifact ("This tool invokes EchoProxy API of EchoService."). Suggested fix: use operation.name() as before.

Observed (same bundle, old → new):

"name":"Echo"        → "name":"EchoProxy"

2. initialize with an unknown or modern protocolVersion answers 2025-03-26 instead of the latest supported version.
Spec (2025-xx negotiation and 2026-07-28 backward-compat guidance): a server that doesn't support the requested version MUST respond with the latest version it supports — 2025-11-25 here. McpSession.negotiate falls back to defaultVersion(). This is the same bug class #1339 fixes on main (there it answered 2024-11-05); since this PR replaces those files, the rule needs to carry over — probably as a "latest handshake version" concept, since modern versions shouldn't be negotiable via initialize either (which the PR gets right — it just picks the wrong handshake version).

initialize @"2026-07-28" → {"protocolVersion":"2025-03-26", ...}   (expected "2025-11-25")

3. Notifications carrying an unsupported _meta version get a -32022 response.
JSON-RPC forbids responding to notifications, and the emitted response has no id member at all. The -32601 path handles id-less messages correctly (NoResponse); the negotiate-time -32022 throw happens before any id check. Repro:

{"jsonrpc":"2.0","method":"notifications/initialized",
 "params":{"_meta":{"io.modelcontextprotocol/protocolVersion":"2027-01-01",
                    "io.modelcontextprotocol/clientCapabilities":{}}}}
→ 200 {"jsonrpc":"2.0","error":{"code":-32022,...}}   (expected: no response)

Smaller observations

  • Legacy initialize capabilities changed shape: completions:{} and logging:{} are now advertised on handshake-era responses (old clients tolerate the extra keys, but responses are no longer byte-stable, and both back onto stub implementations — empty completions, no-op setLevel). Worth deciding deliberately whether stubs should be advertised.
  • Legacy capabilities still advertise tools/prompts listChanged: true although nothing can deliver those notifications over stateless HTTP (modern discover gets this right with empty capability objects).
  • The HTTP status map covers -32601/-32602/-32020/-32021/-32022 but not -32700/-32600 → 400 (the reference SDK maps those too; only reachable from transport adapters that feed decoded-but-invalid frames).
  • McpEngine.execute(JsonRpcRequest, ProtocolVersion) hardcodes McpTransportContext.STDIO; as an HTTP gateway consuming the engine directly we'd love a context-taking overload so transport context stays truthful once something consumes it.
  • toolFilter applies to tools but prompts pass unfiltered (pre-existing; survived the rewrite).

Happy to share our gateway-side adaptation diff or re-run any scenario. We'd also still love your take on the remaining #1337 questions — particularly the cache-hints configuration surface (ttlMs: 0/private constants today; your conformance baseline marks the caching suite as expected-fail, which ties into the same question).

@psp65

psp65 commented Aug 31, 2026

Copy link
Copy Markdown

Update — re-tested at the current head (80840b5): you're right that our run was against Saturday's head (c098a17). At the current head, all three bugs above are fixed and verified end-to-end through our gateway again:

  1. Proxied tool names are back to the delegate names (operation.name() in McpSchemaFactory.createTool) — bundle-loaded services list as Echo again.
  2. initialize with an unknown/modern version now answers 2025-11-25 — and deriving the fallback as "first registered non-stateless protocol that supports initialize" is a nice way to encode "latest handshake version".
  3. Unsupported-version notifications get no response (NoResponse for id-less calls in the negotiate error path).

Full SDK matrix re-passes at the head (1.28.1; 2.0.0 legacy / auto-adopting-modern / pinned 2026-07-28, including tools/call through the proxy pipeline). The "smaller observations" from the original comment still apply at the head: legacy initialize capabilities now advertise completions/logging (stub-backed, and a shape change from the previous engine), legacy still advertises listChanged: true, and the status map lacks -32700/-32600 → 400. The new public execute(McpCall, McpRequestContext) overload largely addresses the transport-context note. The SPI test suggests external protocol extensibility landed too — that answers the "not blocked on us" goal nicely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants